-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG/ENH: cleanup for Timedelta arithmetic #8884
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Regarding |
|
@jreback Yep, those issues are indeed fixed. I added a bunch of tests and everything is passing now. |
@@ -66,6 +66,11 @@ Enhancements | |||
- Added support for ``utcfromtimestamp()``, ``fromtimestamp()``, and ``combine()`` on `Timestamp` class (:issue:`5351`). | |||
- Added Google Analytics (`pandas.io.ga`) basic documentation (:issue:`8835`). See :ref:`here<remote_data.ga>`. | |||
- Added flag ``order_categoricals`` to ``StataReader`` and ``read_stata`` to select whether to order imported categorical data (:issue:`8836`). See :ref:`here <io.stata-categorical>` for more information on importing categorical variables from Stata data files. | |||
- ``Timedelta`` arithmetic returns ``NotImplemented`` in unknown cases, allowing extensions | |||
by custom classes (:issue:`8813`). | |||
- ``Timedelta`` now supports arithemtic with ``numpy.ndarray`` objects of the appropriate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add release notes for all issues fixed (or if 2 apply to the same one, pls list them there)
@shoyer looks good, some minor doc-like fixes. FYI, you need to list each issue sepately with (closes/fixes) in order to have it close them when merged. pls squash and ping when ready |
#5963 was actually already fixed prior to this PR, but I'll note it anyways :). |
Fixes GH8813 Fixes GH5963 Fixes GH5436 If the other argument has a dtype attribute, I assume that it is ndarray-like and convert the `Timedelta` into a `np.timedelta64` object. Alternatively, we could just return `NotImplemented` and let the other type handle it, but this has the bonus of making `Timedelta` compatible with ndarrays. I also added a `Timedelta.to_timedelta64()` method to the public API. I couldn't find a listing for `Timedelta` in the API docs -- we should probably add that, right? Next up would be a similar treatment for `Timestamp`.
a0cfeca
to
7d00a30
Compare
@jreback looks like we're ready. |
BUG/ENH: cleanup for Timedelta arithmetic
thanks |
@@ -66,6 +66,11 @@ Enhancements | |||
- Added support for ``utcfromtimestamp()``, ``fromtimestamp()``, and ``combine()`` on `Timestamp` class (:issue:`5351`). | |||
- Added Google Analytics (`pandas.io.ga`) basic documentation (:issue:`8835`). See :ref:`here<remote_data.ga>`. | |||
- Added flag ``order_categoricals`` to ``StataReader`` and ``read_stata`` to select whether to order imported categorical data (:issue:`8836`). See :ref:`here <io.stata-categorical>` for more information on importing categorical variables from Stata data files. | |||
- ``Timedelta`` arithmetic returns ``NotImplemented`` in unknown cases, allowing extensions | |||
by custom classes (:issue:`8813`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two spaces in the beginning of the line are missing (to align it with ```Time..`). But can be fixed later when doing a clean-up of the whatsnew file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixups here: ff0756f
Fixes #8813
Fixes #5963
Fixes #5436
If the other argument has a dtype attribute, I assume that it is ndarray-like
and convert the
Timedelta
into anp.timedelta64
object. Alternatively, wecould just return
NotImplemented
and let the other type handle it, but thishas the bonus of making
Timedelta
compatible with ndarrays.I also added a
Timedelta.to_timedelta64()
method to the public API. Icouldn't find a listing for
Timedelta
in the API docs -- we should probablyadd that, right?
Next up would be a similar treatment for
Timestamp
.CC @immerrr